Skip to content

fix: avoid login flag for csh shells#12426

Open
mturac wants to merge 2 commits into
continuedev:mainfrom
mturac:fix/issue-12378-tcsh-shell-flag
Open

fix: avoid login flag for csh shells#12426
mturac wants to merge 2 commits into
continuedev:mainfrom
mturac:fix/issue-12378-tcsh-shell-flag

Conversation

@mturac
Copy link
Copy Markdown

@mturac mturac commented May 16, 2026

Summary

  • skip the Unix login-shell -l flag when $SHELL points at csh or tcsh
  • keep the existing login-shell behavior for bash/zsh-style shells
  • cover both the core terminal tool and CLI terminal tool helpers

Fixes #12378

Validation

  • npm run build:local-deps --prefix extensions/cli
  • npm run vitest --prefix core -- tools/implementations/runTerminalCommand.vitest.ts
  • npm test --prefix extensions/cli -- src/tools/runTerminalCommand.test.ts
  • npx prettier --check core/tools/implementations/runTerminalCommand.ts core/tools/implementations/runTerminalCommand.vitest.ts extensions/cli/src/tools/runTerminalCommand.ts extensions/cli/src/tools/runTerminalCommand.test.ts

Summary by cubic

Skip the Unix login-shell flag for csh/tcsh to prevent command failures, while keeping login behavior for bash/zsh. Applies to both the core terminal tool and the CLI helper. Fixes #12378.

  • Bug Fixes
    • Skip -l for csh/tcsh on Unix/macOS and WSL; keep it for bash/zsh; Windows unchanged.
    • Apply logic in both core terminal tool and CLI helper via getShellCommand.
    • Export getShellCommand and add tests to verify csh behavior.

Written for commit 5f6ec1f. Summary will update on new commits. Review in cubic

@mturac mturac requested a review from a team as a code owner May 16, 2026 20:35
@dosubot dosubot Bot added the size:M This PR changes 30-99 lines, ignoring generated files. label May 16, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 16, 2026

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 4 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="core/tools/implementations/runTerminalCommand.vitest.ts">

<violation number="1" location="core/tools/implementations/runTerminalCommand.vitest.ts:861">
P2: Unix-specific test added without platform guard; will fail on Windows runners</violation>

<violation number="2" location="core/tools/implementations/runTerminalCommand.vitest.ts:869">
P2: Global env cleanup is incorrect when original `SHELL` is undefined; assigning `undefined` to `process.env.SHELL` stringifies it to `'undefined'`, potentially leaking a truthy env value into later tests. Use a conditional restore matching the pattern already used in this file for `HOME` and `USERPROFILE` cleanup.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Fix all with cubic | Re-trigger cubic

Comment thread core/tools/implementations/runTerminalCommand.vitest.ts Outdated
Comment thread core/tools/implementations/runTerminalCommand.vitest.ts Outdated
@mturac
Copy link
Copy Markdown
Author

mturac commented May 19, 2026

I have read the CLA Document and I hereby sign the CLA

@mturac
Copy link
Copy Markdown
Author

mturac commented May 19, 2026

recheck

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M This PR changes 30-99 lines, ignoring generated files.

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

Terminal commands fail when $SHELL is set to tcsh due to hardcoded -l shell flag

1 participant